Rawaudio dev#3653
Conversation
|
I'd prefer not to check for the Jamulus version number but rather based on capabilities - we don't have 4.0.0 out yet and it might break during the dev process. |
I wanted to reuse information already available as much as possible so I just added the code where there were version checks already implemented. (For sequence number and pan feature) |
|
Tested it and yes, the noise would be unacceptable. What is our fallback if max is selected but the server doesn't support it? |
I just noticed that if you connect to a server with Max selected you get the noise unless you switch audio quality again while connected. The server code is fine and doesn't need changes, I misplaced the check for my introduced bRawAudioSupported in the client code. I'll have a closer look |
|
Adding a slot to the client to reinit when it receives the server version seems to have fixed the noise issue. |
|
We still get crashes on windows, especially when using more coplex setups including audio routing software. Linux, Mac and android builds work fine so far. Sounds great but still needs more testing and fixes |
|
The last commits fixed the crash on windows and make the client fall back to opus reliably. This is now ready to be tested thoroughly. |
|
A buffersize of 256 on Max quality setting gives garbled audio and the packet sizes seem wrong and contain blocks of zeroes. Only that particular setting is affected. Opus still works |
I plan to try out this enhancement over the next few days. I've had a look through the diffs so far. Could you specify exactly the steps to produce this error? |
This is reproducible with a buffersize of 256 samples only. Edit: The packets become bigger than the MTU allows for on 256 samples buffersize and get fragmented once I corrected the calculation of the packet sizes. Does this mean we need to disable raw audio for buffersizes of 256 or is there some mechanism to receive fragmented packets? |
|
I've just tried a build of Using a buffer size of 10.67ms (256) results in each packet containing two frames of audio, each with its own sequence number. In that setting, I was seeing one packet every 10.67ms coming from the Windows client, but still one packet every 5.33ms coming back from the server. They alternated between having zeros in the first frame and zeros in the second frame. So it could possibly be some issue in Note that the client will encode according to the settings in the Client Settings dialog, but the server will encode according to the information in received in the Talking of which, the codec field in the Lines 484 to 492 in 849e823 So when sending props for raw encoding, it should either use |
This build is not taking into account |
|
Ah, so the issue is that the client is not sending enough data to satisfy the server, and the server is therefore adding in packets of zeros to maintain the data rate. Fragmentation should not be an issue, at least with IPv4, as fragmentation and re-assembly happens transparently at the IP layer. In fact, I don't think it will occur anyway, as the traffic from the server is not fragmented. We should just get packets from the client at 5.33ms instead of 10.67ms. In fact, I've been doing some tests with Wireshark of all the various data rates, qualities and mono/stereo, and it seems that the packet interval is normally half the buffer time specified in the Client Settings. Except when "Small buffers" is not checked, and then 2.67 (64) is exactly the same as 5.33 (128). |
Yes please - I'm building directly from your |
|
I think in I don't have any more time today to try it... |
| } | ||
|
|
||
| const int iOffset = iB * SYSTEM_FRAME_SIZE_SAMPLES * vecNumAudioChannels[iChanCnt]; | ||
| // Recognise a raw audio packet by its size |
There was a problem hiding this comment.
I think it would be better to recognise the audio frame by a sentinel byte. Protocol frames begin with 00 00 and must have a good checksum. Otherwise they are considered to be audio. Opus frames always begin with 00 for mono and 04 for stereo. So maybe for raw audio, the audio data could be prepended with a byte of f0 for mono and f4 for stereo? Then it could be recognised unambiguously. Both client and server need to recognise the format of a received frame correctly without relying on an out-of-band context.
There was a problem hiding this comment.
As far as I understood these sentinel bytes come from the opus codec itself and are not deliberately set by Jamulus as a message ID of sorts. I'd have to overwrite actual audio bytes for that to work with my code. Or am I wrong here?
|
I had misunderstood the packet size calculation and it seems fixed with the last commit. |
I think OPUS and OPUS64 only refer to the setting of small network buffers. It isn't related to the actual opus coding. |
Maybe - I hadn't got around to examining how the value was used in the code. It just felt wrong for the message to state OPUS when it wasn't, and maybe a specific value could also be useful to the server. |
The server isn't aware of the opus quality setting. It is only being sent the packet size and feeds that into the opus codec. There is currently no other way for the server than to determine the codec (or none) by the expected packet sizes for rawaudio. OPUS and OPUS64 refer to 128 or 64 samples internal buffering, no relations to audio quality settings. |
|
Just tried the latest build. It's looking good in Wireshark and sounding good too. No fragmentation either, as the max packet size is only 1068 for stereo, max quality, 10.67ms(256). |
|
Have you guys also tested with small network buffers? This seems to work as well for me and with a huge improvement on latency as well (as expected). Tested on arm64 ubuntu server side and win11 for the client. |
f9b0514 to
583c61b
Compare
Done! (After quite an intense time with git ;) |
|
As a Jamulus server provider I have concerns about my traffic limit if a lot of users make use of the raw audio feature. I would like to have the choice to disable raw audio for my server if the traffic will be too high, similar to "https://github.com/jamulussoftware/jamulus/blob/main/src/main.cpp#L315" I would like to have a command line argument for enabling/disabling this raw audio feature in my server. |
|
What's your data cap? For 1fire currently they say this:
|
|
The rate is twice the "high" rate plus a little. So a server with 10 clients goes from 10x 1Mbps up/down to 10x 2Mbps up/down. Certain server operators, e.g.. choral servers where large numbers of clients is normal and desirable, are potentially going to see this as a threat to ongoing provision of service. So I can see the argument for making it opt in. In fact, having an audio quality argument that capped the rate might be the better approach. That would mean changing the meaning of the new protocol message to report the setting. |
Yes, that is a reasonable idea. A server not configured for raw audio would not send the
How would that work? Raw audio is either off or on at a fixed rate (48kHz 16-bit). There is no rate configuration. |
|
This is why I asked if PCM was going to be an option instead of baked in... |
Then again, keeping the "supports high audio quality label" on various Explorer sites makes perfect sense. People might prefer these. Great suggestion imho.
If i get @pljones right he proposes to extend the message to not carry only a boolean rawaudio support flag but to cap the maximum supported/configured/allowed bitrate and therefore audio quality setting. I don't see much point in a forced downgrade as apparently Jamulus server operators were happy with the current highest possible setting for years now, but from a dev standpoint, this makes sense to me. If it's worth another refactor is more on you guys. @softins, you said you already implemented the new protocol detection logic on explorer, but sadly i can't see it yet in yours servers.php and my own detection apparently does not work as expected. Mind to push that to GitHub? Thank you! My 2c |
You are right but I am thinking a bit further. Maybe I have to leave 1fire someday and the new provider may have more stringent limits. |
|
I can see the point, but that point was actually valid but not accounted for before because theoretically even non-raw servers can exhaust bandwidth. In practice this isn't likely to happen (if it ever did) and looking at other platforms, which have been using a multiple of Jamulus' network rates for years (HPSJam, Jacktrip, Sonobus...), I think it is safe to make this feature an opt-out/on-by-default option since I expect the vast majority of server admins to use raw audio. If Jamulus' bandwidth usage was an actual issue today oracle.com wouldn't be giving away VPS servers for free, so from a practical standpoint I'd say bandwidth issues exist theoretically but are negligible in practice because of today's standards. TL,DR: I'd rather make this on-by-default/opt-out because bandwidth isn't an issue nowadays |
Nowadays you can get better servers than 1fire even offers for free on oracle.com and frankly I can't see a reason for bandwidth to shrink in the future.
I think hosting a server on a private connection shouldn't be recommended and isn't necessary nowadays (see above). |
|
Network utilization the internet is ridiculously low - at least at the ISP(s) i have good ties to - also I do not have any reason to assume it's different in most cases. But I agree that we could indeed introduce an opt out or opt in switch. |
You're right. I committed locally and then deployed and tested, but forgot to push to Github. Sorry, it's been a busy week! It's there now. |
|
@dingodoppelt Sure, I totally agree to you. I just want to have the option to turn it off. BTW, the -F option is opt-in right now. Maybe this should also be changed to opt-out for the next major Jamulus release. |
Regarding the inverse fastupdate switch, that would surely confuse if not upset server operators and is definitely a breaking change. Instead, lot's of software i see simply goes with a |
I second that. I kept my change in line with the existing naming scheme - which doesn't use delimiters between words - while thinking of what @foobarth was thinking. This should be addressed in a new issue or PR. I'm happy to adapt this to any future changes. |
a7fbcf9 to
27d1bfc
Compare
|
Thanks! :-) |
|
Hi All, We have a bug that has been reported via via to me which i believe is directly related to the changes made on this branch. Because this issue does not affect the main upstream 3.12.0 release or older versions, i felt the only place this "bug" could have been introduced was on this branch (probably in the client?) so here is the bug report as the user (assisted by an LLM reported it to me). Audio quality setting resets incorrectly when switching between RawAudio and standard servers Description: Steps to reproduce:
Expected behavior:
Actual behavior: Additional notes: |
I can't reproduce this. For me the client falls back to HIGH, as intended. |
|
Also it would make sense if you could give the exact servers where this occurred. |
I am forwarding both messages to the people that reported this "bug" and if i have an update i will share it here. Good to hear that this might just be user error... I did check the code as well and it does indeed seem to properly handle this scenario so I apologize for the extra noise in this already busy space! |
|
It probably would be useful to have an alert (like the new version notice) that disappears after a short period, if the user-selected quality setting wasn't available.
or something. |
I think this may be overkill. As Tony suggested to call the setting "Max" I think it is clear you get the maximum available audio quality. The client doesn't reflect the server's internal frame size so I guess it is fine to handle this the same way. As my guess is that most of the servers will be using raw audio anyway an extra message would only clutter the code. Users will hear if "Max" is available or not and can then decide to choose a different server if the admin doesn't put an info text into the welcome message. |
Add a new "raw" audio quality setting
This PR adds uncompressed audio ("raw") to the quality settings so there is no Opus compression along the way
Discussion in #3654
This feature improves latency as well. I gained 2ms by using uncompressed audio while having a better audio quality.
CHANGELOG: Add uncompressed audio transmission - dedicated to the memory of Hans Petter Selasky (1982 - 2023)
Does this change need documentation? What needs to be documented and how?
Corresponding PR in jamulussoftware/jamuluswebsite #1133
Checklist